Update to use the microgrid API v0.18 #1283

llucax · 2025-10-16T12:50:08Z

This is a WIP PR, it still needs a lot of cleaning up, but all tests should be passing (minus pylint because I added a few FIXME/TODOs.

This PR is mostly created for having early feedback, and to allow doing some experiments to see how this works in the real world before start cleaning it up. It probably makes more sense to look at the resulting File Changes than going commit by commit at this stage.

@shsms FYI.

This introduces the new microgrid API v0.18. Signed-off-by: Leandro Lucarella <[email protected]>

Signed-off-by: Leandro Lucarella <[email protected]>

The microgrid API now doesn't support not reporting a rated fuse for a grid connection, so we don't need to test for that case anymore. Signed-off-by: Leandro Lucarella <[email protected]>

This introduces the new microgrid API v0.17. Signed-off-by: Leandro Lucarella <[email protected]>

The battery pool bounds calculation is buggy and these tests are wrong (see frequenz-floss#1180). By switching to using ranges of bounds, the buggy behaviour changes and make these tests fail. Fixing them is difficult without switching to using ranges natively first, so we just skip these tests for now. Signed-off-by: Leandro Lucarella <[email protected]>

llucax

I think these are all the TODOs/FIXMEs I wrote when doing the update. It would be good to get some feedback.

llucax · 2025-10-16T12:54:54Z

src/frequenz/sdk/microgrid/_old_component_data.py

+        # FIXME: This might not be true forever, but the service for now seems to send
+        # all metrics with the same timestamp for now.


I think this one can be removed, at least IIRC @tiyash-basu-frequenz confirmed this was going to be like this for the time being. In any case, this is a temporary transitional module, at some point we should start using the telemetry messages as they come from the API.

llucax · 2025-10-16T12:55:40Z

src/frequenz/sdk/microgrid/_old_component_data.py

+        if not samples.states:
+            return class_(component_id=samples.component_id, timestamp=timestamp)
+
+        # FIXME: Maybe we can have more than one, in which case we need to merge them?


I think @tiyash-basu-frequenz also confirmed this was fine for now, but I don't remember if the idea is that more than one could come in the future.

I think for EV chargers, we will have component state and cable state, both of which are necessary for its operation. But I guess @tiyash-basu-frequenz means that they will both be inside states[0].states, which is also a list. We will only not have multiple sets of states.

In that case, we just need to update the warning message below.

llucax · 2025-10-16T12:57:02Z

src/frequenz/sdk/microgrid/_old_component_data.py

+    # FIXME: All of this have now a default of 0.0 because this is what it was done when
+    # we used the API v0.15, as we accessed the fields without checking if the fields
+    # really existed, so the defaul protobuf value of 0.0 for floats was used.
+    # We might beed to review this and if they are not present interpret them as None
+    # instea.


This one is tricky, I guess we can leave it as it is now and will have to come back to it forcibly when removing this temporary adaption layer.

Suggested change

# FIXME: All of this have now a default of 0.0 because this is what it was done when

# we used the API v0.15, as we accessed the fields without checking if the fields

# really existed, so the defaul protobuf value of 0.0 for floats was used.

# We might beed to review this and if they are not present interpret them as None

# instea.

# FIXME: All of this have now a default of 0.0 because this is what it was doing when

# we used the API v0.15, as we accessed the fields without checking if the fields

# really existed, so the default protobuf value of 0.0 for floats was used.

# We might need to review this and if they are not present interpret them as None

# instead.

Also, it is the "adaptation" layer.

llucax · 2025-10-16T12:57:55Z

src/frequenz/sdk/microgrid/_old_component_data.py

+                        # FIXME: We assume only one range is present
+                        # If one bound is None, we assume 0 to match the previous
+                        # behavior of v0.15, but this should eventually be fixed


I guess we'll also fix this when removing this adaption layer, not sure if it is worth adding a separate issue for this one.

llucax · 2025-10-16T12:58:43Z

src/frequenz/sdk/microgrid/_old_component_data.py

I guess we can disable pylint fixme check for this entire module and keep the FIXMEs as they are now to remember to address these issues when removing this adaption layer.

llucax · 2025-10-16T13:00:48Z

src/frequenz/sdk/microgrid/_old_component_data.py

+        # Old code:
+        # return self.component_state not in (
+        #     EVChargerComponentState.AUTHORIZATION_REJECTED,
+        #     EVChargerComponentState.ERROR,
+        # ) and self.cable_state in (
+        #     EVChargerCableState.EV_LOCKED,
+        #     EVChargerCableState.EV_PLUGGED,
+        # )
+        # TODO: Verify this logic is correct


Maybe you can check this @shsms ?

just a comment about the cable states, written below.

llucax · 2025-10-16T13:01:40Z

src/frequenz/sdk/microgrid/_power_distributing/_component_managers/_battery_manager.py

+            component.id
            for component in component_graph.predecessors(battery_id)
-            if component.category == ComponentCategory.INVERTER
+            # TODO: Shouldn't this be (SolarInverter, HybridInverter)?


@shsms ? Maybe it is better to create a separate issue anyways as this was a pre-existing issue.

We were not doing that because there were still a lot of edge configs from the early days that didn't specify the type of the inverter. Maybe things have changed now and we can start with a log message if the inverter type is not set.

llucax · 2025-10-16T13:02:41Z

src/frequenz/sdk/microgrid/_power_distributing/_component_status/_battery_status_tracker.py

+        # TODO: This was using ErrorLevel.CRITICAL to see if an error was critical or
+        # warning. I guess now we use the separate errors and warnings fields.


Can you have a look at this one too @shsms ?

This makes sense I think.

llucax · 2025-10-16T13:16:58Z

src/frequenz/sdk/microgrid/component_graph.py

+        # TODO: This is old validation logic, which should probably be removed
+        if component.id == ComponentId(0) and not isinstance(
+            component, GridConnectionPoint
+        ):
+            issues.append(
+                "Component with ID 0 should be a GridConnectionPoint, "
+                f"but it is a {component!r} instead."
+            )


I think this check is safe to remove. I only found a previous check in an unused method _correct_graph_errors, removed in 635914c. I think I wrote this code before committing that removal.

FYI @tiyash-basu-frequenz @shsms @ela-kotulska-frequenz, I will remove this check unless someone complains.

llucax · 2025-10-16T13:17:50Z

src/frequenz/sdk/timeseries/battery_pool/_methods.py

+                    # TODO: This code is unreacheable, as T can't be None. We need to
+                    # figure out which case this was supposed to handle. Maybe we need
+                    # some sentinel value or to allow None in T.


@shsms maybe you can check this one?

looks like it stopped being T | None here: bf22d6d

The handling for this case can go away as well.

shsms

Only read a little bit of the big commit

shsms · 2025-10-28T10:04:34Z

src/frequenz/sdk/microgrid/_old_component_data.py

+        if not samples.states:
+            return class_(component_id=samples.component_id, timestamp=timestamp)
+
+        # FIXME: Maybe we can have more than one, in which case we need to merge them?


I think for EV chargers, we will have component state and cable state, both of which are necessary for its operation. But I guess @tiyash-basu-frequenz means that they will both be inside states[0].states, which is also a list. We will only not have multiple sets of states.

In that case, we just need to update the warning message below.

shsms · 2025-10-28T10:09:17Z

src/frequenz/sdk/microgrid/_old_component_data.py

+            ValueError: if the given id is unknown or has a different type.
+        """
+        # pylint: disable-next=import-outside-toplevel,cyclic-import
+        from frequenz.sdk import microgrid


why not relative path instead: from .. import microgrid

shsms · 2025-10-28T10:16:35Z

src/frequenz/sdk/microgrid/_old_component_data.py

+    # FIXME: All of this have now a default of 0.0 because this is what it was done when
+    # we used the API v0.15, as we accessed the fields without checking if the fields
+    # really existed, so the defaul protobuf value of 0.0 for floats was used.
+    # We might beed to review this and if they are not present interpret them as None
+    # instea.


Suggested change

# FIXME: All of this have now a default of 0.0 because this is what it was done when

# we used the API v0.15, as we accessed the fields without checking if the fields

# really existed, so the defaul protobuf value of 0.0 for floats was used.

# We might beed to review this and if they are not present interpret them as None

# instea.

# FIXME: All of this have now a default of 0.0 because this is what it was doing when

# we used the API v0.15, as we accessed the fields without checking if the fields

# really existed, so the default protobuf value of 0.0 for floats was used.

# We might need to review this and if they are not present interpret them as None

# instead.

Also, it is the "adaptation" layer.

shsms · 2025-10-28T10:18:24Z

src/frequenz/sdk/microgrid/_old_component_data.py

+                        sample,
+                    )
+
+        self.active_power_per_phase = cast(PhaseTuple, tuple(active_power_per_phase))


wow, amazing this doesn't happen automatically, given that it is just an alias.

shsms · 2025-10-28T12:36:13Z

src/frequenz/sdk/microgrid/_old_component_data.py

+            and bool(
+                {
+                    ComponentStateCode.EV_CHARGING_CABLE_LOCKED_AT_EV,
+                    ComponentStateCode.EV_CHARGING_CABLE_LOCKED_AT_STATION,
+                }
+                & self.states
+                or {
+                    ComponentStateCode.EV_CHARGING_CABLE_PLUGGED_AT_EV,
+                    ComponentStateCode.EV_CHARGING_CABLE_PLUGGED_AT_STATION,
+                }
+                & self.states


we probably need to drop the *_AT_STATION codes. Cable connected to the station doesn't mean the other end of the cable has a car attached.

shsms · 2025-10-28T12:46:31Z

src/frequenz/sdk/microgrid/_power_distributing/_component_status/_battery_status_tracker.py

+        # TODO: This was using ErrorLevel.CRITICAL to see if an error was critical or
+        # warning. I guess now we use the separate errors and warnings fields.


This makes sense I think.

shsms · 2025-10-28T12:56:33Z

src/frequenz/sdk/microgrid/_power_distributing/_component_status/_battery_status_tracker.py

+        # FIXME: There was a semantic change here, before the component data had only
+        # one state, and now it has a set of states, for now we are considering the
+        # component to be working if any of its states is valid. Maybe we need to switch
+        # to the negative and consider a component not working if is in any known
+        # invalid state instead.


Sounds unlikely, unless there's a bug, buf if there are multiple contradictory states for a component for the same timestamp, then we'll have to assume the component is broken.

shsms · 2025-10-28T12:56:55Z

src/frequenz/sdk/microgrid/_power_distributing/_component_status/_battery_status_tracker.py

+        # FIXME: There was a semantic change here, before the component data had only
+        # one state, and now it has a set of states, for now we are considering the
+        # component to be working if any of its states is valid. Maybe we need to switch
+        # to the negative and consider a component not working if is in any known
+        # invalid state instead.


We'll have to assume the component is broken.

shsms · 2025-10-28T12:58:27Z

src/frequenz/sdk/microgrid/_power_distributing/_component_status/_ev_charger_status_tracker.py

+                {
+                    ComponentStateCode.EV_CHARGING_CABLE_LOCKED_AT_EV,
+                    ComponentStateCode.EV_CHARGING_CABLE_LOCKED_AT_STATION,
+                }
+                & ev_data.states
+                or {
+                    ComponentStateCode.EV_CHARGING_CABLE_PLUGGED_AT_EV,
+                    ComponentStateCode.EV_CHARGING_CABLE_PLUGGED_AT_STATION,
+                }
+                & ev_data.states


Here too, we have to drop the AT_STATION states. An ev charger can't be assigned power if it doesn't have an EV attached, and that's what the is_working method is really about.

shsms · 2025-10-28T13:30:53Z

src/frequenz/sdk/timeseries/battery_pool/_methods.py

+                    # TODO: This code is unreacheable, as T can't be None. We need to
+                    # figure out which case this was supposed to handle. Maybe we need
+                    # some sentinel value or to allow None in T.


looks like it stopped being T | None here: bf22d6d

The handling for this case can go away as well.

github-project-automation bot added this to Python SDK Roadmap Oct 16, 2025

github-project-automation bot moved this to To do in Python SDK Roadmap Oct 16, 2025

llucax added 6 commits October 16, 2025 14:52

WIP: Bump dependency for frequenz-client-microgrid to v0.18.0

f54d082

This introduces the new microgrid API v0.18. Signed-off-by: Leandro Lucarella <[email protected]>

Update release notes

c4786d9

Signed-off-by: Leandro Lucarella <[email protected]>

Remove test for grid connection without a fuse

0d01f53

The microgrid API now doesn't support not reporting a rated fuse for a grid connection, so we don't need to test for that case anymore. Signed-off-by: Leandro Lucarella <[email protected]>

WIP: Bump dependency for frequenz-client-microgrid to v0.8.0

04ccf61

This introduces the new microgrid API v0.17. Signed-off-by: Leandro Lucarella <[email protected]>

WIP

b104b88

llucax force-pushed the microgrid-api-0.18 branch from 60cf5b2 to a996c07 Compare October 16, 2025 12:52

llucax commented Oct 16, 2025

View reviewed changes

llucax self-assigned this Oct 16, 2025

llucax moved this from To do to In progress in Python SDK Roadmap Oct 16, 2025

llucax added this to the v1.0.0-rc2200 milestone Oct 16, 2025

shsms reviewed Oct 28, 2025

View reviewed changes

		# FIXME: This might not be true forever, but the service for now seems to send
		# all metrics with the same timestamp for now.

		# TODO: This was using ErrorLevel.CRITICAL to see if an error was critical or
		# warning. I guess now we use the separate errors and warnings fields.

Uh oh!

Update to use the microgrid API v0.18 #1283

Are you sure you want to change the base?

Update to use the microgrid API v0.18 #1283

Uh oh!

Conversation

llucax commented Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llucax left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shsms left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

llucax commented Oct 16, 2025 •

edited

Loading